refactor: Build RBAC with recommended labels#846
Conversation
335474d to
3e60002
Compare
|
| /// `service_account_name` is the name of the RBAC `ServiceAccount` the role-group Pods run under | ||
| /// (RBAC resources are built and applied separately, in the reconcile step). | ||
| /// |
There was a problem hiding this comment.
I can't apply this and I'm not sure what it is referring to.
There was a problem hiding this comment.
I've put each new sentence on a new line, in case that was what was meant.
| /// The type-safe role name for a Druid role. | ||
| pub(crate) fn role_name(role: &DruidRole) -> RoleName { | ||
| RoleName::from_str(&role.to_string()) | ||
| .expect("a DruidRole always serializes to a valid role name") | ||
| } |
There was a problem hiding this comment.
This role_name function is cumbersome to use:
// &DruidRole → &RoleName
&ValidatedCluster::role_name(role)If we add implementations for From right next to DruidRole
impl From<DruidRole> for RoleName {
fn from(value: DruidRole) -> Self {
RoleName::from_str(&value.to_string()) Result<RoleName, Error>
.expect("a DruidRole always serializes to a valid role name")
}
}
impl From<&DruidRole> for RoleName {
fn from(value: &DruidRole) -> Self {
RoleName::from_str(&value.to_string()) Result<RoleName, Error>
.expect("a DruidRole always serializes to a valid role name")
}
}then it would become:
// &DruidRole → &RoleName
&role.into()If we add a Deref<Target = RoleName> for DruidRole
stackable_operator::constant!(COORDINATOR_ROLE_NAME: RoleName = "coordinator");
stackable_operator::constant!(BROKER_ROLE_NAME: RoleName = "broker");
stackable_operator::constant!(HISTORICAL_ROLE_NAME: RoleName = "historical");
stackable_operator::constant!(MIDDLE_MANAGER_ROLE_NAME: RoleName = "middlemanager");
stackable_operator::constant!(ROUTER_ROLE_NAME: RoleName = "router");
impl Deref for DruidRole {
type Target = RoleName;
fn deref(&self) -> &Self::Target {
match self {
DruidRole::Coordinator => &COORDINATOR_ROLE_NAME,
DruidRole::Broker => &BROKER_ROLE_NAME,
DruidRole::Historical => &HISTORICAL_ROLE_NAME,
DruidRole::MiddleManager => &MIDDLE_MANAGER_ROLE_NAME,
DruidRole::Router => &ROUTER_ROLE_NAME,
}
}
}then it would be completely transparent for the user:
// &DruidRole → &RoleName
roleIn this case, ValidatedCluster::recommended_labels can be removed.
There was a problem hiding this comment.
This role_name function is cumbersome to use:
It was a one-liner! I've replaced it with the better one-liner, but I'm not so convinced about the deref: it seems like a lot of code to add for something simple, and ValidatedCluster::recommended_labels can be moved to operator-rs in a later step anyway.
Description
Part of: stackabletech/issues#869
Definition of Done Checklist
Author
Reviewer
Acceptance
type/deprecationlabel & add to the deprecation scheduletype/experimentallabel & add to the experimental features tracker